Skip to content

Conversation

@796RCP92VZ
Copy link
Contributor

@796RCP92VZ 796RCP92VZ commented Oct 12, 2020

Why

Monday.com Ticket

This PR includes the initial form pages for login and signup. They still need error checking, as well as updates on response from the backend (e.g. if the password is correct/incorrect, whether the values in the form are valid, etc.).

This PR

I added a login page and a signup page. This involved creating new .tsx files, creating a new interface to store the signup data, and updating the navbar/router code.

Screenshots

Provide screenshots of any new components, styling changes, or pages.

Verification Steps

I ran a local development instance with npm start. I clicked through the pages and provided various inputs.

@796RCP92VZ 796RCP92VZ changed the title WIP: Signup Add login and sign up pages Oct 12, 2020
@796RCP92VZ 796RCP92VZ changed the title Add login and sign up pages Add login and signup pages Oct 12, 2020
@796RCP92VZ 796RCP92VZ requested a review from jackblanc October 12, 2020 15:38
Copy link
Member

@jackblanc jackblanc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry that I took so long to get back to you on this. There are a few things which could be improved but not required.

Instance.interceptors.request.use(requestInterceptor);

const INVALID_ACCESS_TOKEN: string = 'Given access token is expired or invalid';
const INVALID_ACCESS_TOKEN = 'Given access token is expired or invalid';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛏️ Any reason not to have this type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prettier told me to get rid of it 🤷‍♂️

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait really? What was the error message?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something along the lines of the string type hint being unnecessary because the constant could only be a string / was obviously a string. I don't remember the exact text, unfortunately.

@jackblanc
Copy link
Member

thanks for making those changes, looks like there's a minor eslint issue:

./src/containers/home/Home.tsx
  Line 4:18:  'Checkbox' is defined but never used  @typescript-eslint/no-unused-vars

@796RCP92VZ 796RCP92VZ merged commit 6dd0c5b into master Oct 28, 2020
@796RCP92VZ 796RCP92VZ deleted the login-signup branch October 28, 2020 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants